Block Library: enhance the author block #19894

draganescu · 2020-01-25T13:13:43Z

Description

Closes #19696

Aside from the implementation here and the details in the review, the follwing things are still pending:

always use the display name
add font size to the inspector
add link option in inspector

Things to explore, possibly outside this PR based on feedback:

it is not obvious that changing the author in the block updates the post
inline changing of the author
is byline really needed? It could be a paragraph before it.
multiple authors in the block for one post

Screenshots

How has this been tested?

Enable the full site editing experiment
Create a single template for full site editing
Create a post
Add the author block
Preview the post

draganescu · 2020-01-31T19:36:52Z

@mapk this PR implements most (all?) of the functionality needed by the new author block, but It doesn't look best yet.

One thing though: I added an avatar size selector to the inspector. Using block styles to play with the avatar size, like in the mock-up, would mean that we use the avatar as a background instead of an image, and I think it's not OK. The avatar's URL depends on both the author and the size, so we can't change that based on style (or can we?) 😁

tellthemachines

I'm having trouble testing this PR correctly. In the instructions, where do I "Create a single template for full site editing"? I tried to do so in Appearance > Templates, but can't find out how to apply the resulting template to a post.
If I skip that step, I am able to add a Post Author block to a regular post, but although it displays in the editor, on the front end "No matching template found" is rendered as the post title.

In the editor view, it would be good to have some padding on the left of the block content:

packages/block-library/src/post-author/block-colors-selector.js

packages/block-library/src/post-author/edit.js

packages/block-library/src/post-author/editor.scss

mapk · 2020-02-05T17:48:04Z

Thanks for wrangling this PR, @draganescu! After fiddling with the block a bit, here's some feedback.

The Avatar size is a bit out of context when I don't toggle "on" the avatar. Let's only show the avatar size if the avatar toggle is "on". Something like this:

I have my display name set as "Mapk" so when I toggle "on" the "Show display name" option, I expected to see that, but instead saw my first and last name. When I toggled "off" it showed my display name. This felt like a bug.
There is a lag when the page is loading. The block shows "Post Author Placeholder" instead of just showing the author's name.

The byline, when added, should align where the Author name was before.

I wonder if the byline should mimic the "citation" in the Quote block. So this way it's there ready for the user to type something, but when clicking away from the block, it disappears if nothing is present.

The vertical and horizontal spacing should all be equal in these areas.

Reducing the spacing to 10px helps add a consistent grid throughout. See below:

mapk · 2020-02-05T17:51:18Z

When thinking about the positioning of the byline, I think it might work better aligned with the other text. Maybe even styled to look like the Quote block's citation.

draganescu · 2020-02-11T14:25:57Z

Current state:

shaunandrews · 2020-02-12T20:00:48Z

The text alignment button in the toolbar doesn't seem to do anything, and I'm not sure it should even be there at all. Similar, while the color button does work, I wonder if its needed or required.

I've been trying to find a way to surface some of the options from the sidebar into the toolbar. I've ended up with this "Display" menu:

Its a little generic, but I think it helps "unhide" all these options which are currently in the sidebar.

Also, having the byline on it's own line seems a little much. Often, I'd expect this to literally just contain "Written by:" or similar language, and it could look strange to force two lines when a single line (byline + authorName) seems to work better.

mapk · 2020-02-12T20:18:55Z

I've been trying to find a way to surface some of the options from the sidebar into the toolbar. I've ended up with this "Display" menu:

I'm glad you're exploring ways to bring over these bits to the block. Looking at this, I wonder if we should just call it "Settings" instead of "Display"?

Does this open up the ability to add all settings to the block toolbar? The Accessibility Team, I think, would love that to happen. But before it does, I do struggle with clear rules for when a "Display" dropdown should be used over the Sidebar. Do you have an idea on how we might communicate the difference to developers?

Also, having the byline on it's own line seems a little much.

Your byline works beautifully.

MichaelArestad · 2020-02-12T20:42:37Z

This is a complex block for sure. I wonder if the avatar, byline, name, and bio should all be their own blocks inside the Author block. There is possibly even some merit in being able to add those blocks on their own to get really creative with layout and presentation of the author.

On the one hand, some things get simpler. To remove an avatar, just select it and delete it. To add it back, click the child block inserter. To edit a byline, just click into the field and start typing. To remove it, delete the content of the field. The responsibilities of the parent Author block would then be reduced to style variants and switching the author (maybe?).

Where it gets complicated is how you structure the child blocks. Do you have a very rigid format that prevents the user from moving blocks around? This would enable the styles to be predictable and will have the user relying more on the style variants provided by the theme (if any). If it isn't done with a rigid inflexible layout, will the user have to use things like columns to layout the content inside the Author block? That adds one more layer of complexity.

mapk · 2020-02-13T20:46:36Z

What about an actual icon in the toolbar that toggles the avatar?

I'm still iffy on how the avatar sizing might work though.

shaunandrews · 2020-02-14T16:59:37Z

I wonder if the avatar, byline, name, and bio should all be their own blocks inside the Author block.

I had this same thought early on, using child blocks to define all the elements. But then that would make laying out all the elements more difficult, requiring things like a Columns block to align the profile photo to the left of the name and bio. It still might be worth a exploration, though.

--

Here's another take on using toggle buttons in the toolbar for the profile photo and bio, with an "edit" pencil icon for changing the display name and author:

I added a new interaction for changing the photo size, but it feels a little strange.

mdrovdahl · 2020-02-17T15:37:02Z

Perhaps this thread is not the best place to discuss...but, has any consideration been given to how this block might interplay with https://wordpress.org/plugins/co-authors-plus/?

mapk · 2020-02-17T17:43:04Z

@mdrovdahl, the co-authors option was explored in the issue. I think it's most likely a version 2 for this.

@shaunandrews, I like the further explorations there! I wonder if we need to worry about an avatar size right now. Can the theme handle that instead? I imagine the border-radius would come from the theme as well. Overall, the avatar size option isn't too bad. It borders on being an inner-block at that point, but very limited, which I think is good.

draganescu · 2020-02-18T22:24:43Z

@mapk @shaunandrews I wonder if we could iterate on this block in future PRs? For now the block has the required basic features and most of the interactions suggested here would use the incoming G2.

I can take these suggestions and make new issues out of them following this merge and move on with the current Inspector based interactions this PR implements.

this aloows us to use the sizes as the API serves them

ockham · 2020-04-29T14:18:17Z

Howdy @ockham I pushed a refactoring that removes all storage of author data from the block and leaves only the settings of the block. Also we use the entity save function (great idea! 🙇 ) to update the post when the author is changed (the post has to be saved).

Thanks a ton for making those changes!

Also I have updated the rendering of the block to not use attributes for author data but instead normal author meta WordPress functions.

👍

As for UI I say we merge it like this (UI-wise) and I am sure we can have a future iteration based on some design update. The date block is more fit to that in place style, this one not so (with all those settings). What do you think? We can always add a needs design feedback label, but I wish we could just merge the block since I am sure once used new ideas will appear.

Yeah, I definitely don't want to block this PR any further -- any refinements can be made in follow-up PRs.

I'll give it a last round of testing and will then approve!

packages/block-library/src/post-author/edit.js

ockham · 2020-04-29T14:46:29Z

packages/block-library/src/post-author/edit.js

+	const avatarSizes = [];
+	if ( authorDetails ) {
+		forEach( authorDetails.avatar_urls, ( url, size ) => {
+			avatarSizes.push( {
+				value: size,
+				label: `${ size } x ${ size }`,
+			} );
+		} );
+	}


Could do

Suggested change

const avatarSizes = [];

if ( authorDetails ) {

forEach( authorDetails.avatar_urls, ( url, size ) => {

avatarSizes.push( {

value: size,

label: `${ size } x ${ size }`,

} );

} );

}

let avatarSizes = [];

if ( authorDetails ) {

avatarSizes = Object.keys( authorDetails.avatar_urls ).map( ( size ) => ( {

value: size,

label: `${ size } x ${ size }`,

} ) );

}

-- I personally prefer .map over .forEach since it's a bit more obvious to see that we're mapping one set of values to another.

@ockham 's comment here was marked resolved, but I don't see there there were any revisions. I agree with the point to be using map here. Lodash's map can be slightly more expressive, at least since we're already using forEach from Lodash anyways:

const avatarSizes = map( authorDetails?.avatar_urls, ( url, size ) => ( { value: size, label: `${ size } x ${ size }`, } );

packages/block-library/src/post-author/edit.js

packages/block-library/src/post-author/index.php

ockham

Thanks, this is working nicely!

I left a few suggestions -- mostly syntactic sugar, none of them blocking.

Overall, this is good to merge! 🚢

@ockham

props @ockham

mapk · 2020-04-30T18:29:22Z

Darn, I was hoping to catch this before merging. But I noticed the "byline" section is just too close to the Author's name.

Twenty Twenty

Twenty Nineteen

aduth · 2020-05-05T18:38:30Z

packages/block-library/src/post-author/index.php

+	if ( empty( $attributes ) ) {
+		return '';
+	}


In testing #22114 and rebasing #21696, I'm a bit confused by this condition. A newly inserted Post Author block will have no attributes. But don't we still want to show something for the front-end rendering for a block in this default state?

It's also more obviously a problem in the Site Editor, where none of the Block Inspector fields appear to display for the block to be able to modify the attributes. Is this expected?

This is interesting. I don't remember what was the cause of that condition and it does look weird to me as well, not rendering anything.

On the side of FSE however, that is more nuanced. What you see is a placeholder that exists only if there is no author to show. If there is no author to show, how should the inspector controls behave? Considering they're mostly styling controls ...

packages/block-library/src/post-author/index.php

packages/block-library/src/post-author/edit.js

aduth · 2020-05-14T15:17:24Z

packages/block-library/src/post-author/edit.js

-		[ authorId ]
+import { __ } from '@wordpress/i18n';
+
+const DEFAULT_AVATAR_SIZE = 24;


Is there a reason to assign this in code, rather than as the default value for the attribute?

Good idea, it should be the default attr val.

Good idea, it should be the default attr val.

I looked at it briefly as part of #22359, and there was one potential "gotcha" which could explain why it was treated differently.

At this line of code, we'll only set a value if there's one from attributes, and not consider the default:

gutenberg/packages/block-library/src/post-author/edit.js

Line 139 in a105007

value={ attributes.avatarSize }

I can't say whether that was intentional, but I could see a case to avoid showing the default value as selected (i.e. instead, show nothing as selected until an explicit choice is made). This would behave a lot like the font size picker of Paragraph block, for example.

I still wish it were something we could do as a default attribute instead. Not only does it avoid duplication, it could probably avoid some bugs (e.g. the Post Author avatar currently won't show on the front-end if there is no size selected).

For me, it could be:

Just show the selected value, even if it's the default

Or, hide the value when it's the same as the default

Note: This should consider if the user makes an explicit choice which coincidentally is the same as the default. Technically, these are treated as roughly equivalent, though probably not as expected (Dynamic blocks: default values for attributes aren't converted into html comments #7342).

Or, implement some way to differentiate whether an attribute value was actually sourced from the block's original markup, or if it was provided as a default.

packages/block-library/src/post-author/edit.js

draganescu changed the title ~~Enhances the simple author block~~ Block Library: enhance the simple author block Jan 27, 2020

draganescu mentioned this pull request Jan 27, 2020

New Block: Post Author #19696

Closed

draganescu changed the title ~~Block Library: enhance the simple author block~~ Block Library: enhance the author block Jan 29, 2020

draganescu marked this pull request as ready for review January 31, 2020 16:16

draganescu requested review from ajitbohra, jorgefilipecosta, Soean and talldan as code owners January 31, 2020 16:16

draganescu force-pushed the add/new-block-author branch 2 times, most recently from b1d46cf to 8edda56 Compare February 1, 2020 13:54

draganescu added [Feature] Full Site Editing New Block Suggestion for a new block labels Feb 1, 2020

draganescu force-pushed the add/new-block-author branch from 8edda56 to 4508f91 Compare February 3, 2020 14:56

tellthemachines reviewed Feb 4, 2020

View reviewed changes

mapk added the Needs Design Feedback Needs general design feedback. label Feb 5, 2020

draganescu force-pushed the add/new-block-author branch 2 times, most recently from f74b10f to 7565b18 Compare February 11, 2020 13:25

draganescu added 2 commits April 29, 2020 12:04

linting

1f58cd3

use sizes to label avatar size

9be1ffe

this aloows us to use the sizes as the API serves them

ockham reviewed Apr 29, 2020

View reviewed changes

packages/block-library/src/post-author/edit.js Show resolved Hide resolved

ockham reviewed Apr 29, 2020

View reviewed changes

packages/block-library/src/post-author/edit.js Outdated Show resolved Hide resolved

ockham reviewed Apr 29, 2020

View reviewed changes

packages/block-library/src/post-author/edit.js Show resolved Hide resolved

ockham reviewed Apr 29, 2020

View reviewed changes

packages/block-library/src/post-author/index.php Show resolved Hide resolved

ockham approved these changes Apr 29, 2020

View reviewed changes

draganescu added 2 commits April 30, 2020 10:43

adds some review suggestions

a40a9ef

props @ockham

removes null coalescing operator

ca56f99

draganescu merged commit 2c69473 into master Apr 30, 2020

draganescu deleted the add/new-block-author branch April 30, 2020 11:13

github-actions bot added this to the Gutenberg 8.1 milestone Apr 30, 2020

aduth mentioned this pull request May 5, 2020

PHP notice when adding a Post author block #22100

Closed

aduth reviewed May 5, 2020

View reviewed changes

aduth reviewed May 6, 2020

View reviewed changes

packages/block-library/src/post-author/index.php Show resolved Hide resolved

draganescu mentioned this pull request May 11, 2020

Create a selector/resolver pair for user / author details #22270

Closed

aduth reviewed May 14, 2020

View reviewed changes

packages/block-library/src/post-author/edit.js Show resolved Hide resolved

aduth reviewed May 14, 2020

View reviewed changes

packages/block-library/src/post-author/edit.js Show resolved Hide resolved

aduth mentioned this pull request May 14, 2020

Block Library: Post Author: Refactor edit to use block context #22359

Merged

ellatrix mentioned this pull request Jun 16, 2020

WP 5.5: merge all PHP changes #23210

Closed

12 tasks

mapk mentioned this pull request Jul 27, 2020

Suite of Post Blocks #15623

Closed

8 tasks

mauriac mentioned this pull request Apr 20, 2022

Post Author Block doesn't display when author has no posts #40431

Closed

Mamaduka mentioned this pull request Apr 21, 2022

Block Editor: Deprecate 'BlockColorsStyleSelector' component #40502

Merged

mauriac mentioned this pull request May 7, 2022

Post Author block throw console error when there is no postId #40909

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Block Library: enhance the author block #19894

Block Library: enhance the author block #19894

draganescu commented Jan 25, 2020 •

edited

Loading

draganescu commented Jan 31, 2020

tellthemachines left a comment

mapk commented Feb 5, 2020

mapk commented Feb 5, 2020

draganescu commented Feb 11, 2020

shaunandrews commented Feb 12, 2020

mapk commented Feb 12, 2020

MichaelArestad commented Feb 12, 2020

mapk commented Feb 13, 2020

shaunandrews commented Feb 14, 2020

mdrovdahl commented Feb 17, 2020 •

edited

Loading

mapk commented Feb 17, 2020

draganescu commented Feb 18, 2020

ockham commented Apr 29, 2020

ockham Apr 29, 2020

aduth May 14, 2020

ockham left a comment

mapk commented Apr 30, 2020

aduth May 5, 2020

aduth May 5, 2020

draganescu May 6, 2020

aduth May 14, 2020

draganescu May 15, 2020

aduth May 15, 2020

Block Library: enhance the author block #19894

Block Library: enhance the author block #19894

Conversation

draganescu commented Jan 25, 2020 • edited Loading

Description

Screenshots

How has this been tested?

draganescu commented Jan 31, 2020

tellthemachines left a comment

Choose a reason for hiding this comment

mapk commented Feb 5, 2020

mapk commented Feb 5, 2020

draganescu commented Feb 11, 2020

shaunandrews commented Feb 12, 2020

mapk commented Feb 12, 2020

MichaelArestad commented Feb 12, 2020

mapk commented Feb 13, 2020

shaunandrews commented Feb 14, 2020

mdrovdahl commented Feb 17, 2020 • edited Loading

mapk commented Feb 17, 2020

draganescu commented Feb 18, 2020

ockham commented Apr 29, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ockham left a comment

Choose a reason for hiding this comment

mapk commented Apr 30, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

draganescu commented Jan 25, 2020 •

edited

Loading

mdrovdahl commented Feb 17, 2020 •

edited

Loading